Hey @jay-parikh, thank you very much for your contribution, I really appreciate your effort but I'm afraid this change could be a little bit out of the scope of the library. The main purpose is to rate limit a client, which can be an IP address or a shared resource or whatever you need to control the access of. Adding functionality to not control something would be similar to not calling the rate_limit itself. I suppose that adding some logic in your code to decide whether you need or not to rate_limit would be easier than injecting this responsibility in our library.

Is there any other good reason to implement this feature that I'm missing here? Could you help me to understand more?

Thanks!

Thanks @italorossi for your prompt response. This rate limiting library really comes handy.

Whitelisted client feature comes handy when you have setup Trusted Network which is put behind some Firewall and is sharing same public ip address to communicate with your REST Api endpoint. In this case, we could whitelist client ip of Trusted Network. Which bypass rate limit of Api endpoints.

Or let us say we have an public Api endpoint and we want to rate limit each and every request except our own network (for development team)

Here, we have an option to keep whitelisted clients empty if we want to apply rate limit to each and every incoming request.

Generally if we have look at any popular rate limiting library of any language, whitelisting is a key feature. For instance, https://github.com/fastify/fastify-rate-limit is also having whitelist feature.

Thank you for your contribution, @jay-parikh 🎉
It's a simple and straightforward implementation.

Although I have some considerations:

• The client list is kept in memory, not sure if that scales depending on the number of clients you want to store.
• Also, how are you acquiring that list? Are you querying a relational database? Isn't it slow compared to Redis?
• Maybe this list could be stored on Redis and the library could have methods to add/remove clients to/from it.

But anyway:

• If you have access to a list of clients that shouldn't be bypassed, why don't you just skip rate limit for them?

If you're retrieving max requests for a given client from a database (e.g. Redis or Postgres), yet another interesting approach could be using math.inf as the max_requests argument. This way you could keep track of the client's requests and have a usage history if you'd like to revoke this privilege later. But I don't know if this could be very useful...

Actually, since you've said that you're trying to unconditionally allow requests coming from your own network, you could create a function that returns a boolean value checking if the client's IP address is part of your network (using a netmask, for example). That could be more efficient than having to maintain a list of allowed IP addresses and it's quite feasible depending on your network topology. Based on this function's return you could decide whether to call the rate limit function or not.

I personally agree with @italorossi. I believe this decision is very particular to your business logic and use case. I wouldn't like to force users of this library to adhere to a specific pattern on this matter, especially when it seems to be very simple to come up with your own logic here.

@jay-parikh @italorossi

What do you guys think about skipping the rate limit check if max_requests is None?

• it's a standard way to bypass the limit check
• it still leaves the decision process on the user's hands

Usage example:

from redis_rate_limit import RateLimit, TooManyRequests

ALLOWED_CLIENTS = ('127.0.0.1',)
max_requests = None if client in ALLOWED_CLIENTS else 10

try:
    with RateLimit(resource='users_list', client='192.168.0.10', max_requests=max_requests):
        return '200 OK'
except TooManyRequests:
    return '429 Too Many Requests'

If we're doing this, we need to add documentation specifying that the use case intended is with a very short list of allowed IPs. It's not suited for large sets of clients.

Sorry for the delay!

Yes, the implementation is simple enough to be considered. The Fastify example is a little bit different since it's a plugin to the web framework but anyway...

We can go ahead and implement that with the following changes:

1. whitelisted_clients=[] - two problems here, the name and we shouldn't be using an empty list as default arguments :) suggestion: ignored_clients=None
2. add the ignored_clients check at increment_usage at the top of this function, something like if self.ignored_clients and self.client in self.ignored_clients: return 0
3. please revert all the empty lines removals

About the max_requests I'd prefer to not change it and leave it as a required integer.

Sorry for the delay!

Yes, the implementation is simple enough to be considered. The Fastify example is a little bit different since it's a plugin to the web framework but anyway...

We can go ahead and implement that with the following changes:

1. whitelisted_clients=[] - two problems here, the name and we shouldn't be using an empty list as default arguments :) suggestion: ignored_clients=None
2. add the ignored_clients check at increment_usage at the top of this function, something like if self.ignored_clients and self.client in self.ignored_clients: return 0
3. please revert all the empty lines removals

About the max_requests I'd prefer to not change it and leave it as a required integer.

Thanks for your valuable feedback. I have done all suggested changes. Please review and let me know.

@victor-torres If every thing is all right here then can we merge this pull request?

@victor-torres If every thing is all right here then can we merge this pull request?

I'm just waiting for @italorossi's input regarding moving the verification logic to another method.
See #27 (comment)